Move the SymbolFinder API over to SymbolAndProjectId #18560
Conversation
CyrusNajmabadi
added
the
PR For Personal Review Only
|
Tagging @dotnet/roslyn-ide |
|
Checking in to unblock OOP work. Let me know if there are things you'd like me to change. |
|
|
||
| /// <summary> | ||
| /// Find the declared symbols from either source, referenced projects or metadata assemblies with the specified name. | ||
| /// </summary> | ||
| internal static async Task<ImmutableArray<SymbolAndProjectId>> FindAllDeclarationsAsync( |
There was a problem hiding this comment.
Should this be made public, since the original API that you are marking as legacy was public?
There was a problem hiding this comment.
Ideally, sure. But that would be a ton of work (especially given that SymbolAndProjectId is not public). It is out of scope of this work (afaict) to make this public, and i would not want to gate the OOP work on that.
|
|
||
| #region Current API | ||
|
|
||
| // This region contains the current FindDeclaratins APIs. The current APIs allow for OOP |
| #region Current API | ||
|
|
||
| // This region contains the current FindDeclaratins APIs. The current APIs allow for OOP | ||
| // implementation and will defer to the oop server if it is available. If not, it will |
There was a problem hiding this comment.
OOP should be captilized consistently.
There was a problem hiding this comment.
Although, it may be better to avoid the acronym overall.
| // implementation and will defer to the oop server if it is available. If not, it will | ||
| // compute the results in process. | ||
|
|
||
| internal static async Task<ImmutableArray<SymbolAndProjectId>> FindSourceDeclarationsWithNormalQueryInLocalProcessAsync( |
There was a problem hiding this comment.
Public?
Also, this is the "InLocalProcess" so it isn't clear that the comment makes sense.
There was a problem hiding this comment.
I do not want to make these APIs public yet. At the verry least, i'd want to make sure they were actually the right API to expose. We're stuck in the position that the old APIs are public, and now we can't do anything about that. I don't want to be in that position again.
There was a problem hiding this comment.
The name has been fixed in the later PR. it follow the 'InCurrentProcess' naming pattern.
This enables having htis API run OOP in the future.